Skip to content

Improve single-qubit gate count in TwoQubitControlledUDecomposer#16123

Open
ShellyGarion wants to merge 24 commits intoQiskit:mainfrom
ShellyGarion:two_qubit_control_u_optimize
Open

Improve single-qubit gate count in TwoQubitControlledUDecomposer#16123
ShellyGarion wants to merge 24 commits intoQiskit:mainfrom
ShellyGarion:two_qubit_control_u_optimize

Conversation

@ShellyGarion
Copy link
Copy Markdown
Member

@ShellyGarion ShellyGarion commented May 3, 2026

close #16036

This PR reduces the total number of 1-qubit unitaries needed to synthesize a general 2-qubit unitary using TwoQubitControlledUDecomposer from 24 to 8.

This is in particular useful for the peephole optimization in #13419.

Details:

The TwoQubitControlledUDecomposer consists of the following steps.

  1. We start from a general 4x4 unitary U.

  2. Then we find a cannonical gate W (Weyl gate) s.t.
    U(0,1) = c2r(0) c2l(1) W(0,1) c1r(0) c1l(1)
    this yields 4 1-qubit unitary gates.

     ┌─────┐┌───────┐┌─────┐
q_0: ┤ c2r ├┤0      ├┤ c1r ├
     ├─────┤│  Weyl │├─────┤
q_1: ┤ c2l ├┤1      ├┤ c1l ├
     └─────┘└───────┘└─────┘
  1. Then, we write the Weyl gate W as:
    W(0,1) = RXX(0,1) RYY(0,1) RZZ(0,1)
     ┌─────────┐┌─────────┐        
q_0: ┤0        ├┤0        ├─■──────
     │  Rxx(a) ││  Ryy(b) │ │ZZ(c) 
q_1: ┤1        ├┤1        ├─■──────
     └─────────┘└─────────┘        
  1. Now, we rewrite RYY(0,1) and RZZ(0,1) as:
    RYY(0,1) = sdg(0) sdg(0,1) RXX(0,1) s(0,1) s(0,1)
    RZZ(0,1) = h(0) h(0,1) h(0,1) h(0,1) h(0,1)
    yielding 8 additional 1-qubit unitary gates.
     ┌─────┐┌─────────┐┌───┐
q_0: ┤ Sdg ├┤0        ├┤ S ├
     ├─────┤│  Rxx(b) │├───┤
q_1: ┤ Sdg ├┤1        ├┤ S ├
     └─────┘└─────────┘└───┘
     ┌───┐┌─────────┐┌───┐
q_0: ┤ H ├┤0        ├┤ H ├
     ├───┤│  Rxx(c) │├───┤
q_1: ┤ H ├┤1        ├┤ H ├
     └───┘└─────────┘└───┘
  1. If Equiv is the target controlled-U gate (which is locally equivalent to RXX), then we rewrite RXX using Equiv:
    RXX(0,1) = k2r(0) k2l(1) Equiv(0,1) k1r(0) k1l(1)
    yielding 3*4 1-qubit unitary gates.
     ┌─────┐┌────────┐┌─────┐
q_0: ┤ k2r ├┤0       ├┤ k1r ├
     ├─────┤│  Equiv │├─────┤
q_1: ┤ k2l ├┤1       ├┤ k1l ├
     └─────┘└────────┘└─────┘

This gives a total of:
3*4 + 8 + 4 = 24 1-qubit unitary gates.

     ┌─────┐┌─────────┐┌───────────┐┌─────────┐┌─────┐┌─────────┐┌───────────┐»
q_0: ┤ c2r ├┤ rxx_k2r ├┤0          ├┤ rxx_k1r ├┤ Sdg ├┤ ryy_k2r ├┤0          ├»
     ├─────┤├─────────┤│  Equiv(a) │├─────────┤├─────┤├─────────┤│  Equiv(b) │»
q_1: ┤ c2l ├┤ rxx_k2l ├┤1          ├┤ rxx_k1l ├┤ Sdg ├┤ ryy_k2l ├┤1          ├»
     └─────┘└─────────┘└───────────┘└─────────┘└─────┘└─────────┘└───────────┘»
«     ┌─────────┐┌───┐┌───┐┌─────────┐┌───────────┐┌─────────┐┌───┐┌─────┐
«q_0: ┤ ryy_k1r ├┤ S ├┤ H ├┤ rzz_k2r ├┤0          ├┤ rzz_k1r ├┤ H ├┤ c1r ├
«     ├─────────┤├───┤├───┤├─────────┤│  Equiv(c) │├─────────┤├───┤├─────┤
«q_1: ┤ ryy_k1l ├┤ S ├┤ H ├┤ rzz_k2l ├┤1          ├┤ rzz_k1l ├┤ H ├┤ c1l ├
«     └─────────┘└───┘└───┘└─────────┘└───────────┘└─────────┘└───┘└─────┘

In this PR we multiply the 1-qubit unitary matrices betwen the 2-qubit gates, to get at most 8 1-qubit unitary gates (in the general case where 3 2-qubit gates are needed).

AI/LLM disclosure

  • I didn't use LLM tooling, or only used it privately.
  • I used the following tool to help write this PR description:
  • I used the following tool to generate or modify code:

@ShellyGarion ShellyGarion added this to the 2.5.0 milestone May 3, 2026
@ShellyGarion ShellyGarion requested a review from mtreinish May 3, 2026 08:09
@ShellyGarion ShellyGarion added synthesis Rust This PR or issue is related to Rust code in the repository labels May 3, 2026
@github-project-automation github-project-automation Bot moved this to Ready in Qiskit 2.5 May 3, 2026
@coveralls
Copy link
Copy Markdown

coveralls commented May 3, 2026

Coverage Report for CI Build 25538423326

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.06%) to 87.627%

Details

  • Coverage increased (+0.06%) from the base build.
  • Patch coverage: 6 uncovered changes across 1 file (175 of 181 lines covered, 96.69%).
  • 761 coverage regressions across 17 files.

Uncovered Changes

File Changed Covered %
crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs 181 175 96.69%

Coverage Regressions

761 previously-covered lines in 17 files lost coverage.

Top 10 Files by Coverage Loss Lines Losing Coverage Coverage
crates/circuit/src/dag_circuit.rs 398 84.69%
crates/circuit/src/circuit_drawer.rs 73 95.26%
qiskit/circuit/quantumcircuit.py 71 94.46%
crates/circuit/src/circuit_data.rs 52 87.17%
crates/circuit/src/parameter/parameter_expression.rs 51 91.04%
crates/circuit/src/register_data.rs 42 69.74%
crates/transpiler/src/passes/basis_translator/compose_transforms.rs 16 84.67%
crates/transpiler/src/commutation_checker.rs 14 88.79%
crates/transpiler/src/passes/commutation_cancellation.rs 10 91.38%
crates/circuit/src/interner.rs 9 97.02%

Coverage Stats

Coverage Status
Relevant Lines: 122124
Covered Lines: 107013
Line Coverage: 87.63%
Coverage Strength: 960843.1 hits per line

💛 - Coveralls

@ShellyGarion ShellyGarion marked this pull request as ready for review May 6, 2026 10:03
@ShellyGarion ShellyGarion requested a review from a team as a code owner May 6, 2026 10:03
@qiskit-bot
Copy link
Copy Markdown
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core
  • @levbishop

@ShellyGarion ShellyGarion changed the title [WIP] Improve single-qubit gate count in TwoQubitControlledUDecomposer Improve single-qubit gate count in TwoQubitControlledUDecomposer May 6, 2026
@ShellyGarion ShellyGarion added the Changelog: Added Add an "Added" entry in the GitHub Release changelog. label May 6, 2026
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, thanks for fixing this. I just had a few comments inline.

Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
Comment thread crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs Outdated
@mtreinish mtreinish self-assigned this May 6, 2026
@ShellyGarion ShellyGarion requested a review from mtreinish May 7, 2026 13:22
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, I just had a few more small comments inline.

.collect::<SmallVec<_>>();
(inv_gate.0.into(), inv_gate_params)
}
type TwoQubitGateType = (PackedOperation, SmallVec<[f64; 3]>, SmallVec<[u8; 2]>);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 3rd type in this tuple, the SmallVec<[u8; 2]> is representing qubits right (I'm assuming either [0, 1] or [1, 0]? Is there ever a case where it could be two indices? I'm wondering if we should make this [u8; 2] (no small vec), or just a bool. I'm thinking maybe making this a struct like:

Suggested change
type TwoQubitGateType = (PackedOperation, SmallVec<[f64; 3]>, SmallVec<[u8; 2]>);
struct TwoQubitGateType {
op: PackedOperation,
params: SmallVec<[f64; 3]>,
reversed_qubits: bool,
}

might be better because then we have named access to make it clearer what the params are.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually we only use qubits [0, 1] here, so we don't need any parameter.
I updated the code in 11166c1

let (inv_gate_name, inv_gate_params) = invert_1q_gate(gate);
gates.push((inv_gate_name, inv_gate_params, smallvec![1]));
if !k2l.try_inverse_mut() {
panic!("matrix k2l is not invertible");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was on the fence between asking to use unreachable!() here or not. But I think I agree panic!() is right here. But we should add a little more detail to message, because if we execute this panic this means the underlying error is actually in the TwoQubitWeylDecomposition because it somehow returned a non-unitary matrix for the 1q component which is an internal error in the weyl decomposition.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the panic error in a7e69bc


if !is_inv_rxx {
// 1-qubit gates before the rxx_op, on qubits 0 and 1 respectively
if !k2r.try_inverse_mut() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does something like:

Suggested change
if !k2r.try_inverse_mut() {
if !k_mats[2].try_inverse_mut() {

work here? The reason is we end up building two arrays of Matrix2 right now and if we do it in place on the existing k_mats we avoid creating a second copy of the array of matrices.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that we don't only invert the matrices, but also change their order.
namely, if is_inv_rxx=true then k_mats = [k1r, k1l, k2r, k2l],
and if is_inv_rxx=false then k_mats = [k2r_inv, k2l_inv, k1r_inv, k1l_inv],
so I think I need 2 different arrays in this case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still do it in a single array, just with a couple swap() calls to reorder the elements. So at the very end do k_mats.swap(0, 2) and k_mats.swap(1, 3).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's correct. done in d71c5e5

Comment on lines +245 to +263
let mut c2r = c_mats[0]; // before weyl_gate, qubit 0
let mut c2l = c_mats[1]; // before weyl_gate, qubit 1
let mut c1r = c_mats[2]; // after weyl_gate, qubit 0
let mut c1l = c_mats[3]; // after weyl_gate, qubit 1

let rxx_k2r = rxx_mats[0]; // before RXX(a), qubit 0
let rxx_k2l = rxx_mats[1]; // before RXX(a), qubit 1
let rxx_k1r = rxx_mats[2]; // after RXX(a), qubit 0
let rxx_k1l = rxx_mats[3]; // after RXX(a), qubit 1

let mut ryy_k2r: Matrix2<Complex64>; // before RYY(b), qubit 0
let mut ryy_k2l: Matrix2<Complex64>; // before RYY(b), qubit 1
let mut ryy_k1r: Matrix2<Complex64>; // after RYY(b), qubit 0
let mut ryy_k1l: Matrix2<Complex64>; // after RYY(b), qubit 1

let mut rzz_k2r: Matrix2<Complex64>; // before RZZ(c), qubit 0
let mut rzz_k2l: Matrix2<Complex64>; // before RZZ(c), qubit 1
let mut rzz_k1r: Matrix2<Complex64>; // after RZZ(c), qubit 0
let mut rzz_k1l: Matrix2<Complex64>; // after RZZ(c), qubit 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for a code comment it might be nice to have a little ascii art diagram that shows the structure of the output synthesis to explain these comments. it's clear to me but for future maintainers it might be helpful to explain visually.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have an easy way to generate such diagrams?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use Qiskit! :D

from qiskit.circuit import QuantumCircuit, Gate, Parameter
from qiskit.circuit.library import RXXGate, RZZGate, RYYGate

a = Parameter("a")
b = Parameter("b")
c = Parameter("c")

rxx_a = RXXGate(a)
ryy_b = RYYGate(b)
rzz_c = RZZGate(c)
weyl_gate = Gate(name='Weyl', num_qubits=2, params=[])
c2r = Gate(name='c2r', num_qubits=1, params=[])
c2l = Gate(name='c2l', num_qubits=1, params=[])
c1r = Gate(name='c1r', num_qubits=1, params=[])
c1l = Gate(name='c1l', num_qubits=1, params=[])
k2r = Gate(name='k2r', num_qubits=1, params=[])
k2l = Gate(name='k2l', num_qubits=1, params=[])
k1r = Gate(name='k1r', num_qubits=1, params=[])
k1l = Gate(name='k1l', num_qubits=1, params=[])

circuit = QuantumCircuit(2)
circuit.append(c2r, [0])
circuit.append(c2l, [1])
circuit.append(weyl_gate, [0, 1])
circuit.append(c1r, [0])
circuit.append(c1l, [1])
print(circuit)

controlled_u = QuantumCircuit(2)
controlled_u.append(k2r, [0])
controlled_u.append(k2l, [1])
controlled_u.append(rxx_a, [0, 1])
controlled_u.append(k1r, [0])
controlled_u.append(k1l, [1])
controlled_u.append(k2r, [0])
controlled_u.append(k2l, [1])
controlled_u.append(ryy_b, [0, 1])
controlled_u.append(k1r, [0])
controlled_u.append(k1l, [1])
controlled_u.append(k2r, [0])
controlled_u.append(k2l, [1])
controlled_u.append(rzz_c, [0, 1])
controlled_u.append(k1r, [0])
controlled_u.append(k1l, [1])
print(controlled_u)
     ┌─────┐┌───────┐┌─────┐
q_0: ┤ c2r ├┤0      ├┤ c1r ├
     ├─────┤│  Weyl │├─────┤
q_1: ┤ c2l ├┤1      ├┤ c1l ├
     └─────┘└───────┘└─────┘
     ┌─────┐┌─────────┐┌─────┐┌─────┐┌─────────┐┌─────┐┌─────┐        ┌─────┐
q_0: ┤ k2r ├┤0        ├┤ k1r ├┤ k2r ├┤0        ├┤ k1r ├┤ k2r ├─■──────┤ k1r ├
     ├─────┤│  Rxx(a) │├─────┤├─────┤│  Ryy(b) │├─────┤├─────┤ │ZZ(c) ├─────┤
q_1: ┤ k2l ├┤1        ├┤ k1l ├┤ k2l ├┤1        ├┤ k1l ├┤ k2l ├─■──────┤ k1l ├
     └─────┘└─────────┘└─────┘└─────┘└─────────┘└─────┘└─────┘        └─────┘

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great suggestion (although your picture isn't correct).
I added the pictures of the circuits in 3e39016

@ShellyGarion ShellyGarion requested a review from mtreinish May 8, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Added Add an "Added" entry in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository synthesis

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

Improve single-qubit gate count in TwoQubitControlledUDecomposer

4 participants